Conversation
… readiness endpoint
- ENSRainbow now starts its HTTP server immediately, allowing for non-blocking container startup.
- Introduced a new `GET /ready` endpoint to indicate database readiness, returning `200 { status: "ok" }` once ready, and `503 Service Unavailable` during bootstrap.
- Updated API routes to return structured `ServiceUnavailableError` responses while the database is bootstrapping.
- Replaced the previous Docker entrypoint script with a new command that handles the database download and validation in the background.
- Enhanced the ENSIndexer to wait for the new `/ready` endpoint instead of `/health`.
- Migration note: switch from polling `GET /health` to `GET /ready` for database readiness checks.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 8dbe6df The changes in this PR will be included in the next version bump. This PR includes changesets to release 24 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughHTTP server now starts immediately while database download/validation runs in the background; added Changes
Sequence DiagramsequenceDiagram
actor Client
participant Entrypoint as Entrypoint Command
participant HTTPServer as HTTP Server
participant Bootstrap as DB Bootstrap
participant Database as Database
participant SDK as ENSRainbow SDK
Client->>Entrypoint: start container
Entrypoint->>HTTPServer: create pending server (no DB) and start listening
Entrypoint->>Bootstrap: start background DB bootstrap (abortable)
HTTPServer-->>Client: respond to liveness (/health) 200
Client->>SDK: client.ready() (poll)
SDK->>HTTPServer: GET /ready
HTTPServer-->>SDK: 503 ServiceUnavailable (bootstrapping)
SDK-->>Client: not ready / retry
Bootstrap->>Database: download, extract, validate, open
Bootstrap->>Entrypoint: attachDb(database)
Entrypoint->>HTTPServer: mark server ready
Client->>SDK: client.ready() (retry)
SDK->>HTTPServer: GET /ready
HTTPServer-->>SDK: 200 { status: "ok" }
SDK-->>Client: ready
Client->>HTTPServer: GET /v1/heal/...
HTTPServer->>HTTPServer: isReady() check passes
HTTPServer-->>Client: 200 (DB-backed response)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates ENSRainbow’s startup lifecycle so the HTTP server binds immediately while the prebuilt DB is bootstrapped in the background, adding a dedicated readiness signal (GET /ready) and propagating that readiness concept through the SDK and ENSIndexer.
Changes:
- Add
GET /readyand return structured503 Service Unavailableresponses from DB-dependent routes until the DB is attached. - Introduce a Node-based container entrypoint (
pnpm run entrypoint) that runs DB download/validation asynchronously while serving/healthimmediately. - Update
@ensnode/ensrainbow-sdk(client.ready(), 503 error types) and migrate ENSIndexer readiness polling from/healthto/ready.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/ensrainbow-sdk/src/consts.ts | Add 503 ServiceUnavailable error code constant. |
| packages/ensrainbow-sdk/src/client.ts | Add ready() client method; model 503 service-unavailable responses; exclude transient 503s from heal caching. |
| packages/ensrainbow-sdk/src/client.test.ts | Add tests for client.ready() behavior on 200/503. |
| docs/ensnode.io/src/content/docs/ensrainbow/usage/api.mdx | Document liveness vs readiness and /ready semantics + example 503 body. |
| apps/ensrainbow/src/lib/server.ts | Add “pending” server mode, attachDb, and DbNotReadyError gating when DB isn’t attached. |
| apps/ensrainbow/src/lib/api.ts | Add /ready route and consistent 503 responses for DB-dependent endpoints using a shared helper. |
| apps/ensrainbow/src/commands/server-command.ts | Update API wiring to use a public-config supplier function. |
| apps/ensrainbow/src/commands/server-command.test.ts | Add coverage for /ready and pending/attach lifecycle (503→200 transitions). |
| apps/ensrainbow/src/commands/entrypoint-command.ts | New Node entrypoint that starts HTTP immediately and bootstraps DB in the background. |
| apps/ensrainbow/src/commands/entrypoint-command.test.ts | Add tests for idempotent bootstrap path + pending server smoke test. |
| apps/ensrainbow/src/cli.ts | Add entrypoint CLI command and arg parsing/coercion. |
| apps/ensrainbow/scripts/entrypoint.sh | Remove legacy shell entrypoint (and netcat workaround). |
| apps/ensrainbow/package.json | Add entrypoint script. |
| apps/ensrainbow/Dockerfile | Switch container entrypoint to pnpm run entrypoint; remove netcat dependency. |
| apps/ensindexer/src/lib/ensrainbow/singleton.ts | Poll readiness via client.ready() instead of health(). |
| .changeset/ready-endpoint-bg-bootstrap.md | Release notes + migration guidance for the new readiness endpoint and entrypoint behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect(healthData).toEqual({ status: "ok" }); | ||
| const readyRes = await fetch(`${endpoint}/ready`); | ||
| expect(readyRes.status).toBe(503); | ||
|
|
There was a problem hiding this comment.
Remove the trailing whitespace on this blank line to satisfy formatting/linting (Biome typically flags trailing spaces).
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/ensrainbow/src/cli.ts`:
- Around line 39-46: EntrypointCommandCliArgs currently types
"label-set-version" as number but yargs declares it as type: "string" and then
.coerce("label-set-version", buildLabelSetVersion); update the
EntrypointCommandCliArgs interface to match the actual value lifecycle — either
change "label-set-version" to string if you mean the raw CLI input, or to the
post-coercion branded/parsed type returned by buildLabelSetVersion (e.g.,
LabelSetVersion or string|LabelSetVersion) so the field accurately reflects
pre/post-coerce usage and avoids type confusion when using label-set-version
elsewhere.
In `@apps/ensrainbow/src/commands/entrypoint-command.test.ts`:
- Around line 73-76: The pre-bootstrap /ready assertion is flaky because the
existing-DB bootstrap uses setTimeout(..., 0) and may complete before the fetch;
update the test around readyRes/fetch(`${endpoint}/ready`) and
handle.bootstrapComplete to be deterministic by either (A) removing the
intermediate assertion (do not expect 503 before bootstrapping) and only assert
the final state after awaiting handle.bootstrapComplete, or (B) gate the
bootstrap in the test by injecting a test hook/mock so the bootstrap does not
run until you explicitly release it, then assert readyRes.status is 503 before
releasing and 200 after await handle.bootstrapComplete; refer to the readyRes
variable, fetch(`${endpoint}/ready`) call, and handle.bootstrapComplete when
applying the change.
In `@apps/ensrainbow/src/commands/entrypoint-command.ts`:
- Around line 154-168: The catch path must ensure any opened DB handle and its
files are closed/removed before falling back to re-download: when calling
ENSRainbowDB.open(dbSubdir) capture the returned instance (db), and if
subsequent operations (ensRainbowServer.attachDb(db) or buildDbConfig/
buildEnsRainbowPublicConfig) fail, call db.close() (or the DB instance's proper
close/shutdown method) and remove the dbSubdir files (fs.rm or rimraf
equivalent) before proceeding; update the try/catch so the close+remove runs
when db was successfully opened but later steps threw, and keep the existing
logger.warn message to record the fallback.
- Around line 97-103: The shutdown code awaits httpServer.close() even though
`@hono/node-server` returns a Node http.Server whose close is callback-based; wrap
httpServer.close(...) in a Promise inside the close() function so awaiting
actually waits until the server has finished closing (resolve in the close
callback, reject on error) before proceeding to await ensRainbowServer.close();
update the close function (referencing alreadyClosed, httpServer.close,
ensRainbowServer.close, and logger.info) to use this Promise wrapper so the
shutdown fully waits for active connections to finish.
In `@apps/ensrainbow/src/lib/api.ts`:
- Around line 131-137: The /ready handler currently only checks server.isReady()
(in api.get("/ready")), but the routes /v1/config and /v1/labels/count depend on
publicConfigSupplier() being populated after attachDb(), so /ready can return
200 prematurely; change the readiness check to require both server.isReady() and
that the same config supplier used by the other routes (publicConfigSupplier or
an equivalent "configLoaded" flag/state set after publicConfigSupplier() is
populated in the entrypoint) indicates availability before returning 200, i.e.,
gate api.get("/ready") on both server.isReady() and
publicConfigSupplier/populated-state so clients only proceed when /v1/config and
/v1/labels/count will also be healthy.
In `@apps/ensrainbow/src/lib/server.ts`:
- Around line 245-250: The close() method currently awaits this.db.close() while
isReady() still returns true, allowing concurrent handlers to acquire a DB
handle that is being closed; fix it by capturing the DB handle into a local
variable, immediately flip readiness by setting this.db = undefined and
this._serverLabelSet = undefined, then await the capturedHandle.close(); ensure
you reference close(), this.db, isReady(), and _serverLabelSet when making the
change so readiness is cleared before awaiting the close.
In `@docs/ensnode.io/src/content/docs/ensrainbow/usage/api.mdx`:
- Line 17: Update the API docs to replace all leftover `GET /v1/version` details
with `GET /v1/config` and ensure the documented response matches the
readiness-aware `ENSRainbowPublicConfig` shape: include fields `version`,
`dbSchemaVersion`, `labelSet` (with `labelSetId` and `highestLabelSetVersion`)
and add `recordsCount`. Also change the error handling doc to use
`ServiceUnavailableError` with HTTP 503 for DB bootstrap/not-ready scenarios
(remove the old 500 "database is not initialized" case). Ensure curl examples
and JSON samples use `curl https://api.ensrainbow.io/v1/config` and the updated
JSON response shape.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 11f4de95-3df8-4bb0-9699-37adf1b169a5
📒 Files selected for processing (16)
.changeset/ready-endpoint-bg-bootstrap.mdapps/ensindexer/src/lib/ensrainbow/singleton.tsapps/ensrainbow/Dockerfileapps/ensrainbow/package.jsonapps/ensrainbow/scripts/entrypoint.shapps/ensrainbow/src/cli.tsapps/ensrainbow/src/commands/entrypoint-command.test.tsapps/ensrainbow/src/commands/entrypoint-command.tsapps/ensrainbow/src/commands/server-command.test.tsapps/ensrainbow/src/commands/server-command.tsapps/ensrainbow/src/lib/api.tsapps/ensrainbow/src/lib/server.tsdocs/ensnode.io/src/content/docs/ensrainbow/usage/api.mdxpackages/ensrainbow-sdk/src/client.test.tspackages/ensrainbow-sdk/src/client.tspackages/ensrainbow-sdk/src/consts.ts
💤 Files with no reviewable changes (1)
- apps/ensrainbow/scripts/entrypoint.sh
…-download-database-in-background
- Updated the `/ready` endpoint to require both database attachment and public config population for a successful `200` response. - Adjusted API routes to return `ServiceUnavailableError` for `/v1/heal`, `/v1/labels/count`, and `/v1/config` during bootstrap. - Introduced a grace period for child process termination during shutdown. - Changed the `label-set-version` option type from string to number for better validation.
| await ensRainbowServer.attachDb(db); | ||
| return buildEnsRainbowPublicConfig(await buildDbConfig(ensRainbowServer)); |
- Added support for graceful shutdown during the bootstrap process, allowing in-flight operations to be aborted and child processes to be terminated properly. - Introduced a new `BootstrapAbortedError` to handle shutdown scenarios during database operations. - Updated the entrypoint command to ensure that partially-opened LevelDB handles are closed promptly, preventing resource leaks. - Enhanced the database bootstrap pipeline to respect abort signals, ensuring a clean exit during shutdown.
…shutdown - Introduced a new utility function `closeHttpServer` to handle the closure of HTTP servers gracefully, ensuring all active connections are completed before shutdown. - Updated the entrypoint and server commands to utilize `closeHttpServer`, improving resource management during server shutdown. - Adjusted tests to reflect changes in the readiness check, ensuring consistent behavior during bootstrap.
- Enhanced error handling in the entrypoint command to ensure proper closure of existing database connections when a bootstrap operation fails. - Introduced logic to manage the state of the database connection, allowing for a clean re-download of the database if the initial opening fails. - Updated logging to provide clearer warnings during database operation failures, improving debugging and resource management.
- Updated the `close` method to ensure readiness state is flipped before awaiting the database closure, preventing concurrent handlers from accessing a closing database. - Introduced a local reference to the database for safer closure management, enhancing error handling during shutdown scenarios.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
apps/ensrainbow/src/commands/server-command.ts:56
shutdownis registered directly as a SIGTERM/SIGINT handler but it’s async and can throw/reject (it rethrows after logging). Since Node doesn’t await signal handlers, this can surface as an unhandled promise rejection during shutdown. Prefer wrapping the handler (e.g.() => void shutdown().catch(...)) and avoid rethrowing from the signal path.
const shutdown = async () => {
logger.info("Shutting down server...");
try {
await closeHttpServer(httpServer);
await db.close();
logger.info("Server shutdown complete");
} catch (error) {
logger.error(error, "Error during shutdown:");
throw error;
}
};
process.on("SIGTERM", shutdown);
process.on("SIGINT", shutdown);
} catch (error) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Updated the entrypoint command to resolve the bootstrap promise on shutdown abort, improving error handling during database bootstrap. - Refactored the server command to ensure graceful shutdown by preventing multiple shutdown calls and handling errors more effectively. - Enhanced the `closeHttpServer` utility to treat already stopped servers as a no-op, improving robustness during shutdown scenarios.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…enhance readiness check logic
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…-download-database-in-background
… and related warnings
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…503 in API responses
…-download-database-in-background
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lite PR
Tip: Review docs on the ENSNode PR process
Summary
netcatworkaround from Implement netcat listener for open port scanning on Render #1607).GET /readyreadiness endpoint;/healthremains a pure liveness probe. Core API routes (/v1/heal/:labelhash,/v1/labels/count,/v1/config) return503 Service Unavailableuntil the DB is attached.ensrainbow-sdkexposes aclient.ready()method andENSIndexer's readiness polling now uses it instead ofhealth().Why
netcat-based orchestration workaround from Implement netcat listener for open port scanning on Render #1607 now that the server itself exposes the right probes.ENSRainbowServeris the proper solution requested by the team.Testing
pnpm -F ensrainbow -F @ensnode/ensrainbow-sdk -F ensindexer typecheck— all clean.pnpm test --project ensrainbow— 160/160 pass, including newentrypoint-command.test.tsand new pending-state cases inserver-command.test.ts(asserting/healthis 200 immediately,/readyand core routes are 503 untilattachDb(), then 200/functional).pnpm test packages/ensrainbow-sdk/src/client.test.ts— 25/25 pass, including new 200 and 503 cases forclient.ready().pnpm test --project ensindexer— 201/201 pass, covering the migratedwaitForEnsRainbowToBeReadyretry loop.ENTRYPOINT ["pnpm", "run", "entrypoint"], netcat removed,wget/tarretained) is covered by unit tests that spawn the command and assert the HTTP lifecycle, but a manualdocker compose upsmoke test would be a good reviewer sanity check.Notes for Reviewer (Optional)
ENSRainbowServernow has a "pending" state:createPending()starts without a DB,attachDb(db)transitions to ready,isReady()gates API handlers, andclose()disposes the DB.heal()/labelCount()throwDbNotReadyErrorbefore attach.api.tsgained aPublicConfigSupplierso/v1/configcan lazily read DB-backed config only once the DB is attached; unavailable → sharedbuildServiceUnavailableBody()helper for consistent 503 responses.entrypointCommandreturns anEntrypointCommandHandle { close(), bootstrapComplete }so tests (and future callers) can await the background bootstrap and release the LevelDB lock cleanly. On bootstrap failure itprocess.exit(1).scripts/entrypoint.shis deleted; the Node.js entrypoint spawns the existingdownload-prebuilt-database.shviachild_processand shells out totarfor extraction — no logic duplication.docs/ensnode.io/src/content/docs/ensrainbow/usage/api.mdxwith a "Liveness vs. Readiness" section and/readyexamples. Changeset:.changeset/ready-endpoint-bg-bootstrap.mdwith a migration note for SDK consumers.isCacheableHealResponsewas updated to explicitly excludeHealServiceUnavailableErrorso clients don't cache transient 503s during bootstrap.Pre-Review Checklist (Blocking)